-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CSS import support #139
Conversation
# Conflicts: # examples/screenshot.rs # packages/blitz-dom/src/htmlsink.rs # packages/blitz-dom/src/net.rs
@nicoburns any thoughts about this? If you feel this is useless/unneeded, I would close it. |
@kokoISnoTarget Hello! Apologies for letting this sit for so long. I definitely want this feature, although I would not consider it super-high priority. My main issue with this PR is the introduction of a static variable. I believe it ought to be possible to implement this without it (perhaps using an I also still think we will want to move the "net" crate to a simpler "url in, bytes out" model. And generally simplify the interface between modules (I don't think all the traits we have currently are necessary, but I haven't quite worked out how it would work without them yet). But that doesn't need to be done here. I like how this PR moves the net stuff into a |
Signed-off-by: koko <[email protected]>
# Conflicts: # packages/blitz-dom/src/document.rs
Signed-off-by: koko <[email protected]>
Signed-off-by: koko <[email protected]>
I wouldn't know what would be responsible for the bytes (I guess the DOM or an implementation using the DOM), we'd also lose the capability to parse something like CSS on the "fetch" thread.
If we do the "URL in, bytes out" we could remove |
Let's keep this out of scope of this PR, but:
So I think the idea would be that in the short-term it would just be handled internally by
Exactly. I also think it would be possible to make |
packages/blitz-traits/src/net.rs
Outdated
fn bytes(self: Box<Self>, bytes: Bytes, callback: SharedCallback<Self::Data>); | ||
fn bytes(self: Box<Self>, bytes: Bytes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this changed (callback
is no longer passed in by net crate)? I feel like the Document
/HtmlDocument
/HtmlSink
should never need to know about the callbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed that because it make the API simpler and the callback is only needed when we want to put a resource into the DOM, that does not happen in cases like the @import
rule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this back please? It is definitely simpler if the dom
crate never knows about the callbacks (well, for now, only temporarily in the "resource handlers", but once we make it "url in, bytes out" the dom
won't need the callbacks at all).
Signed-off-by: koko <[email protected]>
Signed-off-by: koko <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's very hard for me to determine exactly what has changed in the util
/net
code due to the file rename. Would perhaps have been better as a separate PR (making things easy to review will definitely get them reviewed faster...). But this new code looks ok to me :)
This adds support for CSS imports.
This also changes the net logic/structure.